Conversation
Codecov Report
@@ Coverage Diff @@
## main #646 +/- ##
==========================================
+ Coverage 55.46% 55.58% +0.11%
==========================================
Files 50 52 +2
Lines 10293 10421 +128
==========================================
+ Hits 5709 5792 +83
- Misses 3722 3761 +39
- Partials 862 868 +6
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
tzdybal
left a comment
tzdybal
left a comment
There was a problem hiding this comment.
Left some comments. You also need to fix tests.
| @@ -0,0 +1,349 @@ | |||
| package node | |||
There was a problem hiding this comment.
I would love to see continuous git history for this code. Ideally full_node.go should be "detected" as moved & edited version of node.go. Probably, node.go you'll need two commits - first with move (node.go->full_node.go), second with creation of new node.go file.
There was a problem hiding this comment.
is there some way i can restore it?
S1nus
requested review from
Manav-Aggarwal,
gupadhyaya,
nashqueue and
tzdybal
| @@ -48,37 +34,26 @@ const ( | |||
|
|
|||
| // Node represents a client node in rollmint network. | |||
There was a problem hiding this comment.
the comment can be updated to represent that Node is interface
| key, _, _ := crypto.GenerateEd25519Key(rand.Reader) | ||
| signingKey, _, _ := crypto.GenerateEd25519Key(rand.Reader) | ||
| node, err := NewNode(context.Background(), config.NodeConfig{DALayer: "mock"}, key, signingKey, abcicli.NewLocalClient(nil, app), &types.GenesisDoc{ChainID: "test"}, log.TestingLogger()) | ||
| node, err := newFullNode(context.Background(), config.NodeConfig{DALayer: "mock"}, key, signingKey, abcicli.NewLocalClient(nil, app), &types.GenesisDoc{ChainID: "test"}, log.TestingLogger()) |
There was a problem hiding this comment.
can this still use NewNode with config.Light set?
| anotherKey, _, _ := crypto.GenerateEd25519Key(rand.Reader) | ||
|
|
||
| node, err := NewNode(context.Background(), config.NodeConfig{DALayer: "mock"}, key, signingKey, abcicli.NewLocalClient(nil, app), &types.GenesisDoc{ChainID: "test"}, log.TestingLogger()) | ||
| node, err := newFullNode(context.Background(), config.NodeConfig{DALayer: "mock"}, key, signingKey, abcicli.NewLocalClient(nil, app), &types.GenesisDoc{ChainID: "test"}, log.TestingLogger()) |
There was a problem hiding this comment.
For this one, it tests things that are specific to FullNodes. I just updated it to use NewNode, but now it casts to FullNode.
| checkTxResCh := make(chan *abci.Response, 1) | ||
| err = c.node.Mempool.CheckTx(tx, func(res *abci.Response) { | ||
| mp := c.node.GetMempool() | ||
| err = mp.CheckTx(tx, func(res *abci.Response) { |
There was a problem hiding this comment.
you can inline the code c.node.GetMempool().CheckTx( since there is only single use for mp
similarly for c.node.GetP2P().GossipTx(
for more than 1 use, you can have a separate variable defined
|
I start to lean into abandoning this PR completely and directly implementing #661. 🤔 |
|
Closing in favor of #681. |
4 participants
Overview
Adds a CLI option for running Rollmint as a light client.
Resolves #642, #651, #650
Checklist